Skip to content

avoid some unnecessary object creations and copies#4493

Merged
danmar merged 6 commits intocppcheck-opensource:mainfrom
firewave:copy
Sep 29, 2022
Merged

avoid some unnecessary object creations and copies#4493
danmar merged 6 commits intocppcheck-opensource:mainfrom
firewave:copy

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

No description provided.

@danmar danmar merged commit eeb6db0 into cppcheck-opensource:main Sep 29, 2022
@firewave firewave deleted the copy branch September 29, 2022 20:44
@firewave
Copy link
Copy Markdown
Collaborator Author

This regressed the callgrind step in the CI from 85,414,145,406 to 85,422,952,324.

@firewave firewave restored the copy branch October 7, 2022 16:00
@firewave firewave deleted the copy branch October 7, 2022 16:01
@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Oct 7, 2022

The regression is caused by 92c08d1.

In ErrorMessage::toString() there is a readCode() call which didn't happen before.

This happens because the ErrorMessage::FileLocation constructor is setting ErrorMessage::FileLocation::mOrigFileName whereas ErrorMessage::FileLocation::setfile() isn't.

ErrorMessage::FileLocation::setfile() also does sanitization of the path but the constructor doesn't.

@danmar Is this intentional?

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Oct 9, 2022

This happens because the ErrorMessage::FileLocation constructor is setting ErrorMessage::FileLocation::mOrigFileName whereas ErrorMessage::FileLocation::setfile() isn't.

Spontanously my feeling is that this might be intentional. But I'd need to dig into the code to know.

ErrorMessage::FileLocation::setfile() also does sanitization of the path but the constructor doesn't.

not sure if sanitization is needed. but if it is then the constructor should probably also do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants